[PM-40488] fix: clear clipboard setting does not clear the clipboard on Android 13+ - #7169
[PM-40488] fix: clear clipboard setting does not clear the clipboard on Android 13+#7169mvanhorn wants to merge 3 commits into
Conversation
|
Thank you for your contribution! We've added this to our internal tracking system for review. Details on our contribution process can be found here: https://contributing.bitwarden.com/contributing/pull-requests/community-pr-process. |
bitwarden-bot
changed the title
| ) | ||
| }, | ||
| ) | ||
| if (isBuildVersionAtLeast(version = Build.VERSION_CODES.P)) { |
There was a problem hiding this comment.
Do we need this version check here?
There was a problem hiding this comment.
Can you remove this since we now have tests
Remove the isBuildVersionAtLeast(P) gate and the OmitFromCoverage annotation now that the worker has test coverage.
|
Both removed in 5e2465c: the version check is gone and OmitFromCoverage dropped now that the tests cover the worker. |
| clipboardManager = context.getSystemService(Context.CLIPBOARD_SERVICE) as ClipboardManager | ||
| worker = ClearClipboardWorker( | ||
| appContext = context, | ||
| workerParams = mockk(relaxed = true), |
There was a problem hiding this comment.
Can this be done inline above?
private val context = RuntimeEnvironment.getApplication()
private val clipboardManager = context.getSystemService<ClipboardManager>()
private val worker: ClearClipboardWorker = ClearClipboardWorker(
appContext = context,
workerParams = mockk(relaxed = true),
)or do these tests require a new instance each time?
Per review, fold the @before setup into property initializers. getSystemService<T>() returns T?, so the clipboard manager is unwrapped with requireNotNull rather than !!, which detekt's UnsafeCallOnNullableType rejects. Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
|
Pushed a0302ac with the inline setup you suggested. One deviation from your snippet: On your other two comments, I believe both are already addressed and GitHub has them marked outdated:
If either of those is not what you meant, point me at the current line and I will fix it. Verified locally: |
| appContext.getSystemService(CLIPBOARD_SERVICE) as ClipboardManager | ||
|
|
||
| override fun doWork(): Result { | ||
| clipboardManager.setPrimaryClip( |
There was a problem hiding this comment.
To make sure I'm on the up-and-up... The idea is to set "nothing" to the clipboard, overwriting what sensitive information was there, and then invoke .clearPrimaryClip (removing the new empty content) as per Android platforms on which the method is not a no-op.
Is this correct?
|
Yes, that's the intent. setPrimaryClip replaces whatever sensitive content was on the clipboard with an empty clip flagged IS_SENSITIVE, so the data is gone from the clipboard regardless of what happens next. clearPrimaryClip then removes that empty entry so the clipboard isn't left holding a blank item, on platforms where it actually does something. The overwrite is the part that protects the data; the clear is cleanup. |
|
Love it. Thanks for the breakdown!
All the best,
Franco N. Colaizzi
…On Thu, Sep 3, 2026 at 2:18 AM Matt Van Horn ***@***.***> wrote:
*mvanhorn* left a comment (bitwarden/android#7169)
<#7169 (comment)>
Yes, that's the intent. setPrimaryClip replaces whatever sensitive content
was on the clipboard with an empty clip flagged IS_SENSITIVE, so the data
is gone from the clipboard regardless of what happens next.
clearPrimaryClip then removes that empty entry so the clipboard isn't left
holding a blank item, on platforms where it actually does something. The
overwrite is the part that protects the data; the clear is cleanup.
—
Reply to this email directly, view it on GitHub
<#7169?email_source=notifications&email_token=ABN6AIENIZBOFQMITC3OSCT5NEEJTA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKNJSGE2DCNRQHEZ2M4TFMFZW63VHMNXW23LFNZ2KKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#issuecomment-5521416093>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABN6AIGL3OESL4L3Y2GYUNL5NEEJTAVCNFSNUABEKJSXA33TNF2G64TZHM2TONBVGIYTIMZ3JFZXG5LFHM2DQOJYHE4DONZRGOQXMAQ>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
4 participants
🎟️ Tracking
Fixes #7069
📔 Objective
The "clear clipboard" setting does not clear the clipboard on Android 13+. Setting a short interval (e.g. 10 seconds), copying a password, and waiting leaves the secret on the clipboard. Multiple users have confirmed this across recent devices (Samsung Galaxy S25+ on Android 16, Fairphone FP5 on Android 15, Samsung SM-A366B on Android 16) and app versions 2026.2.1 through 2026.6.x.
The clear is scheduled by
BitwardenClipboardManagerImpl.setText()as a delayedOneTimeWorkRequestand executed byClearClipboardWorker.doWork(), which relies solely onclipboardManager.clearPrimaryClip(). On newer Android versionsclearPrimaryClip()can be a no-op, so the secret is never evicted.This change makes
ClearClipboardWorker.doWork()clear robustly: it first overwrites the primary clip with an empty, sensitiveClipData(markingClipDescription.EXTRA_IS_SENSITIVEon API 33+, and the legacyandroid.content.extra.IS_SENSITIVEkey below that, matching the pattern already used inBitwardenClipboardManagerImpl.setText), then callsclearPrimaryClip()(guarded to API 28+) to remove the placeholder entry on devices that honor it. Background clipboard restrictions on Android 10+ gate reads, not writes, so the background overwrite/clear remains permitted. The fix stays entirely within the existing worker, with no scheduling or DI changes.A unit test (
ClearClipboardWorkerTest) covers the Android 13+ path to prevent regression.📸 Screenshots
Not applicable - this is a background-worker/behavioral fix with no UI surface.
AI was used for assistance.